Skip to content

Send permission denied error#108

Closed
fvacek wants to merge 1 commit into
mainfrom
permission-denied-error
Closed

Send permission denied error#108
fvacek wants to merge 1 commit into
mainfrom
permission-denied-error

Conversation

@fvacek
Copy link
Copy Markdown
Contributor

@fvacek fvacek commented Apr 11, 2026

Do not send a neutral error message so an unauthorized user wouldn't even know that method exists

Do not send a neutral error message so an unauthorized user wouldn't
even know that method exists
@fvacek fvacek requested a review from j4r0u53k April 11, 2026 20:42
@fvacek fvacek self-assigned this Apr 11, 2026
Copy link
Copy Markdown
Collaborator

@j4r0u53k j4r0u53k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a strong objection to the changes in error messages introduced in this PR.

The error message should remain unchanged specifically to avoid leaking information to unauthorized users. If the response differs depending on whether a given method or path exists, it allows an attacker to distinguish between “non-existent” and “unauthorized but valid” endpoints. This effectively enables probing and enumeration of the API surface.

Keeping the error message consistent is a deliberate security measure—it ensures that an unauthorized user cannot determine whether a particular path or method actually exists.

Additionally, this change modifies externally observable behavior, which constitutes a breaking change. However, there is no corresponding major version bump, which violates semantic versioning expectations and may negatively impact downstream users.

For these reasons, I cannot approve this PR in its current form.

@j4r0u53k
Copy link
Copy Markdown
Collaborator

This is definitely a regression that shouldn't get approved by any sane person 👎

@j4r0u53k j4r0u53k closed this Apr 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants